-
Notifications
You must be signed in to change notification settings - Fork 750
feat(api): expose MouseCursor and set_mouse_cursor in public API #9330
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Conversation
Move MouseCursor into internal/common/enums and export via items module Add set_mouse_cursor to WindowAdapter trait Add Window::set_mouse_cursor wrapper Update imports Closes slint-ui#3905
- Fix slint-docsnapper headless.rs implementation - Fix i-slint-backend-qt qt_window.rs implementation - Move set_mouse_cursor from WindowAdapterInternal to WindowAdapter trait - Resolves build errors in CI/CD pipeline
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Thanks for the PR.
Could you please try to cleanup the patch to avoid unrelated change.
Also remove the Window::set_mouse_cursor API, as this is not how one should set a cursor to the window. (It is anyway going to be overridden by the internals)
Forward, | ||
} | ||
|
||
/// This enum represents different types of mouse cursors. It's a subset of the mouse cursors available in CSS. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This didn't need to be moved. Makes the patch harder to review.
/// This enum represents different types of mouse cursors. It's a subset of the mouse cursors available in CSS. | ||
/// For details and pictograms see the [MDN Documentation for cursor](https://developer.mozilla.org/en-US/docs/Web/CSS/cursor#values). | ||
/// Depending on the backend and used OS unidirectional resize cursors may be replaced with bidirectional ones. | ||
enum MouseCursor { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The enum needs to be #[non_exhaustive]
for compatibility with potential future cursor.
/// Sets the mouse cursor for this window. | ||
pub fn set_mouse_cursor(&self, cursor: crate::items::MouseCursor) { | ||
self.0.window_adapter().set_mouse_cursor(cursor); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This API shouldn't need to be there.
In order to set a mouse cursor on the window, one would use a TouchArea with a mouse-cursor property.
KeyEventResult, MouseEvent, | ||
}; | ||
use crate::item_rendering::{CachedRenderingData, ItemRenderer}; | ||
use crate::items::MouseCursor; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
(a bit inconsistent to have some import through crates::items
and some through super
)
use crate::items::MouseCursor; | ||
|
||
#[test] | ||
fn test_mouse_cursor_api() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice test, but maybe it could be in a module related to mouse cursor (items
or platform
) this tests.rs
module is actually about API used by the tests but not the tests themselves.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
unrelated file
- Add #[non_exhaustive] attribute to MouseCursor enum for future compatibility - Add exhaustive pattern matching with wildcard fallback in Winit backend - Add exhaustive pattern matching with wildcard fallback in Qt backend - Ensures compilation with non-exhaustive enum variants Addresses all maintainer feedback from PR review.
Summary
Expose
MouseCursor
in the public API and moveset_mouse_cursor()
to the publicWindowAdapter
trait, so custom backends can set the cursor.Changes
MouseCursor
intointernal/common/enums
(now exported viaitems
module)set_mouse_cursor()
toWindowAdapter
traitWindow::set_mouse_cursor
wrapperMouseCursor
is now used viacrate::items
)Notes
Closes #3905
Usage